Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump React to v16 alpha #445

Merged
merged 6 commits into from
Apr 18, 2017
Merged

Bump React to v16 alpha #445

merged 6 commits into from
Apr 18, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 18, 2017

This pull request seeks to update our React dependency to track the latest alpha version, currently 16.0.0.9-alpha.9. Doing so will allow us to stay ahead of the curve in anticipation of upcoming changes. As the plugin becomes more widely distributed, we'll want to shift this back to a less unstable version, but I'd argue it's fine to opt in to an alpha dependency for our alpha plugin (we're also using the TinyMCE nightly version).

There appear to be no breaking changes with this update, though it comes with a few additions we'll want to take advantage of. The "Fiber" reconciliation algorithm is the main headline for this release, but for our purposes, we're mostly interested in newly supported return values for components: strings and arrays. This will help, in #409 (comment) for example, in avoiding unnecessary wrapper elements in a save return value.

@aduth aduth added the Framework Issues related to broader framework topics, especially as it relates to javascript label Apr 18, 2017
@aduth aduth requested a review from youknowriad April 18, 2017 13:39
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to merge this when the production script is fixed

index.php Outdated
@@ -35,8 +35,9 @@ function gutenberg_register_scripts() {
$suffix = SCRIPT_DEBUG ? '' : '.min';

// Vendor
wp_register_script( 'react', 'https://unpkg.com/react@15/dist/react' . $suffix . '.js' );
wp_register_script( 'react-dom', 'https://unpkg.com/react-dom@15/dist/react-dom' . $suffix . '.js', array( 'react' ) );
$react_suffix = ( SCRIPT_DEBUG ? '.development' : '' ) . $suffix;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we fallback to .production to avoid errors

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we fallback to .production to avoid errors

Yes, added in e25e732.

@aduth
Copy link
Member Author

aduth commented Apr 18, 2017

There's now an unmet peer dependency from react-textarea-autosize which has stagnated for a while now. Was the reason we have React in package.json because of this dependency? We could leave it as the old version if so. In general its presence there is a little confusing.

@youknowriad
Copy link
Contributor

Maybe we could just drop this dependency for now (or replace it with our own implementation)?

@aduth
Copy link
Member Author

aduth commented Apr 18, 2017

I bumped the package.json back down to latest, since it's clearly only necessary for dependencies which define it as a required peer. The Webpack externals configuration should still ensure that it references the window global (16+). It wasn't entirely necessary, but I swapped react-autosize-textarea with react-textarea-autosize since it seems to be more maintained (at least resolved PropTypes warning). There's minimal differences between these components, so I'm not worried if we'd need to revert back to the other module, a different one, or our own custom solution in the future.

@aduth aduth merged commit aa7fc94 into master Apr 18, 2017
@aduth aduth deleted the update/react-16.alpha branch April 18, 2017 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants